Skip to content

Conversation

@mdtoguchi
Copy link
Contributor

When using -fsycl-device-only with the new offloading model, update the behaviors to properly restrict the compilation flow to only produce the device binary. This more tightly integrates with the existing --offload-device-only option that is commonly used.

mdtoguchi added 2 commits May 6, 2024 15:03
When using -fsycl-device-only with the new offloading model, update the
behaviors to properly restrict the compilation flow to only produce the
device binary.  This more tightly integrates with the existing
--offload-device-only option that is commonly used.
@mdtoguchi mdtoguchi marked this pull request as ready for review May 6, 2024 22:08
@mdtoguchi mdtoguchi requested a review from a team as a code owner May 6, 2024 22:08
@mdtoguchi mdtoguchi requested a review from asudarsa May 6, 2024 22:08
@mdtoguchi mdtoguchi temporarily deployed to WindowsCILock May 6, 2024 22:20 — with GitHub Actions Inactive
StringRef ObjSuffix = isMSVCEnv ? ".obj" : ".o";
StringRef NewObjSuffix = isMSVCEnv ? ".new.obj" : ".new.o";
bool Ret =
(ObjFileName.starts_with("libsycl-") &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to deviate from the old-offloading-model flow where we do not seem to be calling Backend? If yes, is that agreeable? Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is agreeable. The functionality is the same, we just introduce the additional phase to better match with the new model expectations.

@asudarsa asudarsa requested a review from hdelan May 7, 2024 18:07
@asudarsa
Copy link
Contributor

asudarsa commented May 7, 2024

Failures seem to be due to some infrastructure issue and unrelated to this change.

Copy link
Contributor

@asudarsa asudarsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks

@asudarsa
Copy link
Contributor

asudarsa commented May 7, 2024

Hi @mdtoguchi

Can you please look at the cuda test fails?

Thanks

@asudarsa
Copy link
Contributor

asudarsa commented May 8, 2024

Hi @mdtoguchi

Can you please look at the cuda test fails?

Thanks

Cuda fails are unrelated to this PR and a fix has been submitted for review here: #13692

Copy link
Contributor

@hdelan hdelan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the ping. LGTM

@mdtoguchi
Copy link
Contributor Author

@intel/llvm-gatekeepers, this looks ready for merge - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants